test(a11y): lock in reduced-motion scroll + focus-not-obscured regressions#1048
Conversation
…sions Guard the two accessibility fixes shipped in #1036 against regression: - ANIM-01 (WCAG 2.3.3): a live Chromium test intercepts the behaviour argument every scripted scrollTo/scrollIntoView receives, then triggers a known scroll ("New chat"). Reduced motion must produce instant "auto" jumps; no-preference must animate ("smooth") — verifying resolveScrollBehavior() is threaded through the call sites, not just unit-tested in isolation. - A11Y-FOCUS-01 (WCAG 2.4.11): a deterministic source pin asserts #main-content reserves scroll-padding-bottom in both mobile dock branches (plus the answer scroll-padding-top). The reservation only renders in app states impractical to reach reliably in a browser test, so it is pinned at source — the repo's established pattern for phone scroll-geometry invariants. Verified: typecheck, lint, format:check, full unit+jsdom suite (348 files / 3108 passed / 0 failed), and both targeted tests green against the local server. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds tests for ClinicalDashboard scroll-padding invariants and verifies scripted scrolling uses ChangesScroll accessibility validation
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/dashboard-scroll-padding.test.ts`:
- Around line 33-37: Update the test assertions around the reserve string in the
dashboard scroll-padding test to verify the matching content-padding pair
appears in both dock branches, rather than only asserting one occurrence. Keep
the existing occurrence count and ensure each reservation is paired with
max-sm:pb-[var(--mobile-composer-reserve)].
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f1230468-c80a-48d8-b81e-9d0b0f8fd2ee
📒 Files selected for processing (2)
tests/dashboard-scroll-padding.test.tstests/ui-accessibility.spec.ts
…t-focus-motion-123366
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2c0898d9da
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…oth dock branches Address the review findings on the A11Y-FOCUS-01 source pin: - Bind every assertion to the extracted `<main id="main-content">` opening tag instead of counting source-wide occurrences, so a reservation class that drifts onto a child or a non-scrolling wrapper now fails the guard (a `>` inside a className comment means the tag is read up to its line-final `>`). - Require the *paired* `pb` + `scroll-padding-bottom` reservation in both dock branches (>=2), so one branch losing its content padding no longer passes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LRZ1EyBZW1ADXrZvMxEEsC
Summary
Regression coverage for the two accessibility fixes shipped in #1036, so they cannot silently regress. Test-only — no product code changes.
ui-accessibility.spec.tswrapsElement.prototype.scrollTo/scrollIntoViewto record thebehaviorargument each scripted scroll receives, then triggers a known one ("New chat"). Under a reduced-motion preference every scripted scroll must be an instant"auto"jump; with no preference the same action animates ("smooth"). This verifiesresolveScrollBehavior()is actually threaded through the call sites, not merely unit-tested in isolation.dashboard-scroll-padding.test.ts) asserts#main-contentreservesscroll-padding-bottomin both mobile dock branches (and the answer-viewscroll-padding-top). The reservation only renders in app states that are impractical to reach reliably in a browser test (an answer-with-content view requires a mocked streaming answer), so it is pinned at source — the repo's established pattern for phone scroll-geometry invariants (seemobile-interaction-regressions.test.ts).Verification
npm run typecheck,npm run lint,npm run format:checknpm run test— full unit + jsdom suite: 348 files / 3108 passed / 0 failed (includes the newdashboard-scroll-padding.test.ts)npm run verify:ui— full Chromium UI lane green (265 passed), including the new ANIM-01 reduced-motion scroll testRisk and rollout
🤖 Generated with Claude Code
Summary by CodeRabbit